@mickaelaccensi two quick comments:
1: Can you put back the extra info about testing in the PR template?
2: Can you explain why the regression test is not included in this PR?

This sounds good! A long time ago (Issue #210) I considered a possible solution to this bug because I was looking at those parts of the code when a user reported it on the ww3 mailing list. I didn't really have interest in the QP (used for the Benjamin-Feir instability index). But I later found that it is still applied in some academic studies, so it is important. I remember it was a bit complex to reach a complete fix of the bug - the bug is present at more than one location in the code.

I've updated the PR message and created an issue for QP bugfix.
I've merged it with the last develop branch, I'm running now the matrix since my pdlib issue seems to be solved (both parmetis and scotch works now on my HPC).
I'll provide you the matrix.comp files for tomorrow.

Hi @mickaelaccensi, I don't see a regtest. Is there a reason you haven't added one?

it is because I had to split the Fabrice branch into 3 and the regtest is designed to work with the next feature branch

it is because I had to split the Fabrice branch into 3 and the regtest is designed to work with the next feature branch

This is adding an output of QKK? I see this in the code, but not in the text of the PR. I get that this is part of larger work, but I'm concerned we aren't testing features when they're added. Could we add the output request of QKK to an existing regression test or 2 (maybe 1 structured 1 unstructured), even temporarily?

added and tested in ww3_tp2.6 and ww3_tp2.15

I would tend to say that it is due to the bug #1062, I'm working on it right to provide a separated PR
could you try to run matrix with inp on your side to confirm that the only differences are the expected ones in ww3_tp2.6 and ww3_tp2.15 ?

could you try to run matrix with inp on your side to confirm that the only differences are the expected ones in ww3_tp2.6 and ww3_tp2.15 ?

Sure, I will run the matrix to get a reference on what is failing and what is not.

I would tend to say that it is due to the bug #1062, I'm working on it right to provide a separated PR

If just using ww3_multi.nml is causing the issue you saw for #1062, then that is fine and it can be resolved later. If it's a symptom of a greater issue -- if it is potentially affecting the diffs for tp2.21, tp2.17 -- then we should get to the bottom of what is going on prior to the PR being merged.

Update: some of the tests submitted yesterday reached time limits. Our primary RD machine was unavailable then, but is back today. I've resubmitted the runs and should have the results later today.

Hi @mickaelaccensi,
Here's the logs from matrix.comp

**********************************************************************
********************* non-identical cases ****************************
**********************************************************************
## known non-b4b
mww3_test_03/./work_PR1_MPI_e                     (1 files differ)
mww3_test_03/./work_PR3_UQ_MPI_e_c                     (1 files differ)
mww3_test_03/./work_PR3_UNO_MPI_e                     (1 files differ)
mww3_test_03/./work_PR2_UNO_MPI_e                     (1 files differ)
mww3_test_03/./work_PR2_UNO_MPI_d2                     (16 files differ)
mww3_test_03/./work_PR1_MPI_d2                     (12 files differ)
mww3_test_03/./work_PR3_UNO_MPI_d2_c                     (14 files differ)
mww3_test_03/./work_PR3_UQ_MPI_d2_c                     (15 files differ)
mww3_test_03/./work_PR3_UNO_MPI_d2                     (12 files differ)
mww3_test_03/./work_PR2_UQ_MPI_d2                     (16 files differ)
mww3_test_03/./work_PR3_UQ_MPI_e                     (1 files differ)
mww3_test_03/./work_PR3_UQ_MPI_d2                     (16 files differ)
ww3_tp2.10/./work_MPI_OMPH                     (7 files differ)
ww3_tp2.16/./work_MPI_OMPH                     (4 files differ)
ww3_ufs1.3/./work_a                     (3 files differ)
##

ww3_tp2.15/./work_PR3_UQ_RHO                     (5 files differ)
ww3_tp2.15/./work_PR3_UQ_RHO_MPI                     (5 files differ)
ww3_tp2.15/./work_ST6FLX5                     (5 files differ)
ww3_tp2.15/./work_ST4FLX5                     (5 files differ)
ww3_tp2.6/./work_ST0                     (4 files differ)
ww3_tp2.6/./work_ST4                     (16 files differ)
ww3_tp2.6/./work_pdlib                     (4 files differ)

**********************************************************************
************************ identical cases *****************************
**********************************************************************

matrixCompSummary.txt
matrixCompFull.txt
matrixDiff.txt

Please add the required regtest so we can proceed with the review. Thanks

Hi Mat,

I've already added the QP and QKK outputs in ww3_tp2.6 and ww3_tp2.15, we should find them in work_ST4

I've already added the QP and QKK outputs in ww3_tp2.6 and ww3_tp2.15, we should find them in work_ST4

Hi Mickael, okay I see. Thanks for that.

Then the only thing outstanding is updating the relevant scripts in /model/inp/ and model/nml/ to reflect this. Could you add documentation for QKK to these scripts?

good catch ! it's now done

Great, thanks for making those changes @mickaelaccensi. I'll work on finishing the review.

@mickaelaccensi

Accounting of non-b4b differences

The first four files of each test below are the same (log files and 1 binary gridded output), and their differences are
accounted for by the QP and QKK variable outputs. The ww3_tp2.15 file ww3.201403.nc is also due to these variable outputs.

This leaves just the 12 additional netCDF files seen below in ww3_tp2.6/work_ST4/. The difference in these files turned out to be in the list of WW3 switches in the global_attributes. In particular, NCEP2 is replaced by NOGRB. I believe this is sneaking in at the call to run_cmake_test where the -o both option has been added, and will affect a condition within run_cmake_test that sed's NCEP2 for NOGRB.

Was the -o both option intended just for debugging, or did you want it as part of the PR?

ww3_tp2.15/./work_PR3_UQ_RHO                       (5 files differ)
ww3_tp2.15/./work_PR3_UQ_RHO_MPI                   (5 files differ)
ww3_tp2.15/./work_ST6FLX5                          (5 files differ)
ww3_tp2.15/./work_ST4FLX5                          (5 files differ)
-------------------------------------              ----------------------
  1)  log.ww3 
  2)  out_grd.ww3
  3)  ww3_ounf.out
  4)  ww3_outf.out

  5)  ww3.201403.nc



ww3_tp2.6/./work_ST0                                 (4 files differ)
ww3_tp2.6/./work_pdlib                               (4 files differ)
-------------------------------------              ----------------------
  1)  log.ww3 
  2)  out_grd.ww3
  3)  ww3_ounf.out
  4)  ww3_outf.out



ww3_tp2.6/./work_ST4                                  (16 files differ)
-------------------------------------              ----------------------
  1)  log.ww3
  2)  out_grd.ww3
  3)  ww3_ounf.out
  4)  ww3_outf.out

  5)  ww3.201008_cfd.nc
  6)  ww3.201008_cfx.nc
  7)  ww3.201008_cge.nc
  8)  ww3.201008_cha.nc 
  9)  ww3.201008_dtd.nc
 10)  ww3.201008_fc.nc
 11)  ww3.201008_hs.nc
 12)  ww3.201008_lm.nc
 13)  ww3.201008_t01.nc
 14)  ww3.201008_t02.nc
 15)  ww3.201008_t0m1.nc
 16)  ww3.201008_ust.nc

Hi @MatthewMasarik-NOAA ,

I've added QKK as output field in ww3_outf.F90 to be consistent and to allows ww3_tp2.6 and ww3_tp2.15 to output QP and QKK as ASCII output. So I've removed the -o both which was intented to output netCDF. However I don't catch your remark about the global attributes. NCEP2 is not present in the switch files for ww3_tp2.6 and ww3_tp2.15, so I don't understand why it should appear in the global attributes. Let me know.

I've added QKK as output field in ww3_outf.F90 to be consistent and to allows ww3_tp2.6 and ww3_tp2.15 to output QP and QKK as ASCII output.

This is sounds good, thanks for making that update.

So I've removed the -o both which was intented to output netCDF.

Great, this should remove the differences from ww3_tp2.6/work_ST4.

However I don't catch your remark about the global attributes. NCEP2 is not present in the switch files for ww3_tp2.6 and ww3_tp2.15, so I don't understand why it should appear in the global attributes.

Yes, I found this confusing too. I believe that run_cmake_test uses sed on the switchfile to change between NOGRB and NCEP2.

develop: ww3.201008_cfd.nc

bugfix/QP: ww3.201008_cfd.nc

I'll re-run the regtests and let you know the result.

@mickaelaccensi Thank you for this nice PR. Extra thanks for the manual documentation!

great ! let me know when you can check and merge the branch bugfix/nml, I have then 2 others PR to send about ST4 and GQM ;)

I will be able to start working on the bugfix/nml PR, likely tomorrow. I'm finishing some work today on #1034 and will be able to start it right after.